Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Thermopro TP828B Meat Thermometers 2 probes #3085

Merged
merged 7 commits into from
Nov 10, 2024

Conversation

ProfBoc75
Copy link
Collaborator

Relate to issue #3082

  • This model TP828B is close to the other model TP829B.
  • Previous decoder have been renamed and updated accordingly.

@zuckschwerdt
Copy link
Collaborator

Most things in the decoder exist twice, once for the TP829B and once for the TP828B.
I'm not sure, it might be easier to use two decoders? Keep one thermopro_tp82xb.c but make it two r_device and two decoder functions, each much shorter and without the branching.
What do you think?

@ProfBoc75
Copy link
Collaborator Author

Most things in the decoder exist twice, once for the TP829B and once for the TP828B. I'm not sure, it might be easier to use two decoders? Keep one thermopro_tp82xb.c but make it two r_device and two decoder functions, each much shorter and without the branching. What do you think?

Yes, I will try, my last version of the decoder can be simplified, as the first data part until temp probe 1 is the same, after we have a gap.

I can also consolidate into a single data_t *data = data_make ... and put some DATA_CON to show the good model & good values too.

  • keep the model guess part
  • extract the common data (id, flag, display unit, temp probe 1)
  • Depends on the model , extract the data accordingly.
  • Output all data into a single data_make list with DATA_CON for model, probes, lo and hi target temp ... like it's already the case in fact (except model) because probe(s) can be removed in that case 0xedd is set by default so I exclude this value.

I'm busy, so will try later, your proposition too...

@zuckschwerdt
Copy link
Collaborator

Thanks! To clarify: the idea is to have two very simple decoders, not because it's less code, but because it's then a very simple flow without any branching or DATA_COND. ("clever" decoders are hard to maintain later on…)

Generally I think: If we want a different "model" (because the protocol is somewhat different) then we should try to have multiple decoders. And if we want the same "model" (because only some data bits are different) then we should try to have only one decoders.

@zuckschwerdt
Copy link
Collaborator

This looks much nicer and easier to follow, I think. Thanks!
The file info block should only contain licence info. If you like lines 10-15 to stay then put them in just a /* */ block (i.e. not /**).
The first doc-comment line (21 and 137) should end with a dot (.) :)

@ProfBoc75
Copy link
Collaborator Author

This looks much nicer and easier to follow, I think. Thanks! The file info block should only contain licence info. If you like lines 10-15 to stay then put them in just a /* */ block (i.e. not /**). The first doc-comment line (21 and 137) should end with a dot (.) :)

I tried to do the same as for acurite.c where lot of decoders ... and for this one, no dot also at the end of the first line ... not a good example ... I was also wondering about that, now you confirmed my doubt.

@ProfBoc75 ProfBoc75 merged commit 8900b18 into master Nov 10, 2024
18 checks passed
@ProfBoc75 ProfBoc75 deleted the feat-thermopro_tp828b branch November 14, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants